Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

turf-points-within-polygon: Allow MultiPolygons in the types #1116

Merged
merged 3 commits into from
Nov 22, 2017
Merged

turf-points-within-polygon: Allow MultiPolygons in the types #1116

merged 3 commits into from
Nov 22, 2017

Conversation

woutervh-
Copy link
Contributor

@woutervh- woutervh- commented Nov 22, 2017

The implementation of turf-points-within-polygon allows MultiPolygon features to be passed, but the types only allow Polygon features. This fix will allow both types.

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Have read How To Contribute.
  • Run npm test at the sub modules where changes have occurred.
  • Run npm run lint to ensure code style at the turf module level.

@DenisCarriere
Copy link
Member

Added a few new JSDocs @link:

  • {@link Points}
  • {@link (Multi)Point}
  • {@link (Multi)Points}
  • {@link LineStrings}
  • {@link (Multi)LineString}
  • {@link (Multi)LineStrings}
  • {@link Polygons}
  • {@link (Multi)Polygon}
  • {@link (Multi)Polygons}

@DenisCarriere DenisCarriere merged commit dc27a2e into Turfjs:master Nov 22, 2017
@DenisCarriere
Copy link
Member

Maybe we should also allow for Feature<Polygon|MultiPolygon> to be allowed as well... Having to create a FeatureCollection for the sake of creating a FeatureCollection is a bit ridiculous.

var searchWithin = turf.featureCollection([
    turf.polygon([[
        [-46.653,-23.543],
        [-46.634,-23.5346],
        [-46.613,-23.543],
        [-46.614,-23.559],
        [-46.631,-23.567],
        [-46.653,-23.560],
        [-46.653,-23.543]
    ]])
]);
//=points
var ptsWithin = turf.pointsWithinPolygon(points, searchWithin);

@woutervh-
Copy link
Contributor Author

Hi @DenisCarriere , not sure why this was assigned to me. It looks like everything in this PR was merged to master including your suggestion about allowing Features.

@DenisCarriere
Copy link
Member

Don't worry about the Assigned, this is mostly to keep track on who did what when we publish the next release.

Makes it easier to see the contributors on the closed PR/Issues.

image

https://github.com/Turfjs/turf/milestone/13?closed=1

And then when we publish a minor/major release we add contributors to the release notes:

#932

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants